Add read-only GitHub Issues access to agent enclaves - #55531
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bcc38c7d-af99-4f49-8c17-b56cbb4515c5
There was a problem hiding this comment.
Pull request overview
Adds isolated, read-only GitHub Issues access for agent enclaves through a dedicated mcpg proxy.
Changes:
- Adds
issues-read-v1schemas, validation, and version gates. - Generates proxy policy, lifecycle, credential isolation, and tests.
- Documents routes, DIFC behavior, and dependency requirements.
Show a summary per file
| File | Description |
|---|---|
.changeset/enclave-github-issues-profile.md |
Records the new profile. |
.github/aw/enclaves.md |
Adds authoring guidance. |
actions/setup/sh/start_enclave_github_proxy.sh |
Starts and configures the proxy. |
actions/setup/sh/stop_enclave_github_proxy.sh |
Cleans up proxy resources. |
docs/src/content/docs/reference/enclaves.md |
Documents profile behavior. |
docs/src/content/docs/reference/glossary.md |
Updates enclave terminology. |
pkg/constants/version_constants.go |
Defines dependency minimums. |
pkg/parser/schema_test.go |
Tests frontmatter validation. |
pkg/parser/schemas/main_workflow_schema.json |
Adds user-facing schema syntax. |
pkg/workflow/awf_env.go |
Excludes proxy handoff variables. |
pkg/workflow/compiler_yaml_ai_execution.go |
Adds proxy cleanup lifecycle. |
pkg/workflow/enclave_github_proxy.go |
Builds policy and lifecycle steps. |
pkg/workflow/enclave_github_proxy_test.go |
Tests proxy integration. |
pkg/workflow/enclaves.go |
Adds configuration and validation. |
pkg/workflow/enclaves_test.go |
Tests AWF configuration output. |
pkg/workflow/mcp_setup_generator.go |
Starts the proxy during MCP setup. |
pkg/workflow/schemas/awf-config.schema.json |
Adds AWF schema support. |
schema-demos/schema-demo-enclaves.md |
Demonstrates the new syntax. |
Review details
Suppressed comments (1)
actions/setup/sh/start_enclave_github_proxy.sh:66
- A cancelled prior run can leave
proxy-tls/ca.crthere. Because the readiness probe usescurl -k, it can accept the new proxy while retaining the stale CA, after which AWF receives a CA that cannot authenticate the proxy. Remove the previous container and log/TLS directory before recreating it.
mkdir -p "$MCP_LOG_DIR"
chmod 700 "$MCP_LOG_DIR"
docker rm -f "$CONTAINER_NAME" >/dev/null 2>&1 || true
- Files reviewed: 18/18 changed files
- Comments generated: 2
- Review effort level: Balanced
|
/matt |
|
/review |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Request changes
This introduces two blocking regressions: the new repo-limit validation rejects valid mixed script + agent.github.cli enclave configs, and the enclave GitHub proxy teardown can be skipped when later host-side steps fail.
Blocking themes
- The
issues-read-v1non-public repository limit is being enforced against the wrong scope, so existing mixed-enclave workflows break as soon as they opt into the new profile. - The proxy cleanup path is not robust against downstream failures, which leaves the capability handoff and proxy state alive longer than the design claims.
🔎 Code quality review by PR Code Quality Reviewer · pi · gpt54 · 9.7 AIC · ⌖ 7.12 AIC · ⊞ 7K
Comment /review to run again
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design and /tdd — overall LGTM with two minor observations (no blocking issues).
📋 Key Themes & Highlights
Key Themes
- Architecture is clean and well-scoped: the PAT/capability key never reaches the AWF sandbox or agent, the proxy runs in bridge mode with no published host port, and the stop step always clears the key from
GITHUB_ENV. The security boundary is deliberately layered and the test suite enforces the contract. - Version gating is conservative: provisional minimums (AWF v0.28.6, MCPG v0.4.11) are separated from global defaults until release artifacts exist. The
validateEnclaveGitHubIssuesVersionsfunction correctly uses the default MCPG version as the fallback when none is specified — meaning omittingsandbox.mcp.versionfails validation. - Two minor observations posted as inline comments:
enclaveGitHubIssuesOperationsas a package-level slice makes profile-to-operations lookup implicit; worth a map when a second profile arrives.- A test case for the nil-MCP-config rejection path would complete the version-gate coverage.
Positive Highlights
- ✅
TestEnclaveGitHubProxyScriptsEnforceDedicatedBridgeContractis an excellent contract-enforcement test — it pins security-critical shell invariants (no-phost port, bridge mode, capability masking ordering) directly in Go. - ✅
TestCompileEnclaveGitHubProxyLifecycleverifies end-to-end compilation ordering and exclusion of all handoff vars from the AWF command line. - ✅
effectivePrimaryGitHubIntegrityFloorcorrectly prefersParsedToolsover raw map access and falls back toapproved— the priority chain is explicit and tested. - ✅ Refactoring
validateEnclavesConfigintovalidateEnclaveEntry/validateEnclaveRepositories/validateEnclaveGitHubIssuesVersionsmeaningfully improves testability and readability.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet46 · 78.7 AIC · ⌖ 10.2 AIC · ⊞ 7.6K
Comment /matt to run again
|
Please do one focused follow-up pass:
Run: https://github.com/github/gh-aw/actions/runs/32787277683
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed in efa862c. I added focused regression coverage for mixed-enclave repository-limit scoping and for unconditional enclave GitHub proxy teardown ( |
|
Please do one focused follow-up pass:
Run: https://github.com/github/gh-aw/actions/runs/32795790586
|
Follow-up done. I replied in the blocking github-actions review threads with concrete fix references to efa862c for both items ( |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
@copilot resolve merge conflicts by discarding all pr updates to .lock.yml files |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Resolved. I merged |
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
Warning Firewall blocked 4 domainsThe following domains were blocked by the firewall during workflow execution:
[!TIP] tools:
github:
mode: gh-proxySee GitHub Tools for more information on To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"
- "api.github.com"
- "chatgpt.com"
- "github.com"See Network Configuration for more information.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
There was a problem hiding this comment.
Request changes
The new enclave GitHub path still leaks the repository catalog into generated workflow YAML via the proxy policy template step env, which undercuts the main isolation guarantee this feature is trying to add.
Blocking theme
- The compiler currently embeds the full
ENCLAVE_GITHUB_PROXY_POLICY_TEMPLATEJSON in theStart Enclave GitHub Proxystep, so assigned private repo names and sensitivities are exposed to the host job before the launcher masks them.
Everything else I checked in the changed lines looked consistent with the intended defer/startup and timeout fixes.
🔎 Code quality review by PR Code Quality Reviewer · pi · gpt54 · 21.2 AIC · ⌖ 7.43 AIC · ⊞ 7K
Comment /review to run again
🏗️ ADR required — draft added for PR #55531I enforced the design-decision gate for this PR because the prefetch summary shows 902 new lines in default business-logic directories, which is above the 100-line threshold. Evidence used
Action takenI added a draft ADR here:
Inferred decision captured in the ADR
Next action for the authorPlease review and refine the draft ADR so it accurately reflects the intended long-term architecture and trade-offs before merge.
|
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
I found two small simplification opportunities in the new enclave-GitHub proxy wiring: one around an unused workflowRunID argument and one around single-caller step helpers. net: -18 lines possible.
Warning
Firewall blocked 4 domains
The following domains were blocked by the firewall during workflow execution:
ab.chatgpt.comapi.github.comchatgpt.comgithub.com
[!TIP]
api.github.com is blocked because GitHub API access uses the built-in GitHub tools by default. Instead of adding api.github.com to network.allowed, use tools.github.mode: gh-proxy for direct pre-authenticated GitHub CLI access without requiring network access to api.github.com:
tools:
github:
mode: gh-proxySee GitHub Tools for more information on gh-proxy mode.
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "ab.chatgpt.com"
- "api.github.com"
- "chatgpt.com"
- "github.com"See Network Configuration for more information.
Generated by ✂️ Ponytail Reviewer for #55531 · codex · mai10 · 15.6 AIC · ⌖ 0.445 AIC · ⊞ 14K
Comment /ponytail to run again
| }, | ||
| } | ||
|
|
||
| func buildEnclaveGitHubProxyPolicyJSON(workflowData *WorkflowData, workflowRunID string) (string, error) { |
There was a problem hiding this comment.
pkg/workflow/enclave_github_proxy.go:L49: yagni: buildEnclaveGitHubProxyPolicyJSON takes a workflowRunID parameter that the compiler always passes as "". Drop the parameter and keep the policy builder focused on the static compile-time policy.
| return string(GitHubIntegrityApproved) | ||
| } | ||
|
|
||
| func (c *Compiler) generateStartEnclaveGitHubProxyStep(yaml *strings.Builder, workflowData *WorkflowData) error { |
There was a problem hiding this comment.
pkg/workflow/enclave_github_proxy.go:L108: yagni: generateStartEnclaveGitHubProxyStep and generateStopEnclaveGitHubProxyStep are only called once each and mostly wrap a few writes. Inline that YAML emission into the surrounding compiler flow to keep the feature local.
There was a problem hiding this comment.
Review: Add read-only GitHub Issues access to agent enclaves
This PR introduces a well-scoped, security-conscious feature: a closed issues-read-v1 profile that gives agent enclaves read-only Issues REST access through a compiler-owned bridge-mode mcpg proxy. The layered isolation design (PAT held only in proxy, per-call awf-egh1 capability, --exclude-env on all proxy artifacts, if: always() teardown) is solid.
One non-blocking comment filed (line 30 of enclave_github_proxy.go): enclaveGitHubProxyMaxTTL = 600 needs a comment explaining that it is a per-invocation TTL, not a per-run TTL, to avoid confusion now that maxEnclaveTimingBucketSeconds has grown to 4800.
No blocking issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet46 · 117.4 AIC · ⌖ 9.74 AIC · ⊞ 6.2K
| WorkflowRunID string `json:"workflow_run_id"` | ||
| Profile string `json:"profile"` | ||
| Audience string `json:"audience"` | ||
| Repositories []enclaveGitHubPolicyRepository `json:"repositories"` |
There was a problem hiding this comment.
Non-blocking — clarify TTL intent relative to the 4800 s bucket.
enclaveGitHubProxyMaxTTL = 600 was introduced in the same PR that raised maxEnclaveTimingBucketSeconds from 600 → 4800. The agent enclave timeout can now be up to 4,740 seconds, but each mcpg-issued awf-egh1 capability has a max TTL of 600 seconds. This is safe only if AWF mints a fresh capability for each enclave GitHub API call — a fact that is currently implicit.
A short comment would prevent future maintainers from wondering whether this value needs to track the bucket:
// enclaveGitHubProxyMaxTTL is the maximum TTL for a single awf-egh1 invocation
// capability minted by AWF. AWF issues a fresh capability per API call, so this
// does not need to match maxEnclaveTimingBucketSeconds.
const enclaveGitHubProxyMaxTTL = 600@copilot please address this.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /codebase-design — requesting changes on two issues identified in the current head after prior review rounds were resolved.
📋 Key Themes & Highlights
Issues Found
-
Capability TTL too short for long enclaves (
enclaveGitHubProxyMaxTTL = 600): the proxy's issued token expires after 10 minutes, butmaxEnclaveTimingBucketSeconds = 4800allows 80-minute runs. Long-running enclave tasks will hit mid-run authorization failures. The fix is to derive the constant from the same timing constants as the tool timeout (see inline comment onenclave_github_proxy.go:22). -
Policy template logged unmasked in Actions UI (
enclave_github_proxy.go:128): the compiled.lock.ymland the Actions job log will render the repository names and sensitivity labels from the policy template JSON before the shell script's::add-mask::line runs. The prior comment at line 131 addressed in-script masking, but the env-block rendering window remains. At minimum, document this in the enclave docs (see inline comment).
Positive Highlights
- ✅ All previous review-round issues (stop-script path, min-version constants, mixed-enclave repo counting, token chain, TLS readiness probe) were systematically addressed and confirmed in follow-up commits — excellent iteration discipline.
- ✅
is_deferred_serverallowlist guard is tight: arbitrary server names cannot be deferred viaGH_AW_MCP_DEFERRED_SERVERS, with four dedicated tests proving the boundary. - ✅
getEffectiveEnclaveGitHubToken()correctly excludesGITHUB_TOKENwith clear reasoning, and the explicit test confirms the chain. - ✅
validateEnclaveRepositoriesis cleanly extracted into a testable helper with independent tests for the scoping fix.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet46 · 142.2 AIC · ⌖ 10.8 AIC · ⊞ 7.6K
Comment /matt to run again
| enclaveGitHubProxyNetworkAlias = "awf-enclave-github-proxy" | ||
| enclaveGitHubProxyAudience = "gh-aw-enclave-github" | ||
| enclaveGitHubProxyPort = 18443 | ||
| enclaveGitHubProxyMaxTTL = 600 |
There was a problem hiding this comment.
[/diagnosing-bugs] enclaveGitHubProxyMaxTTL = 600 (10 minutes) will expire mid-run for enclave tasks that run longer than 600 seconds — but maxEnclaveTimingBucketSeconds = 4800 allows up to 80 minutes. Once the capability TTL lapses, the proxy will refuse further Issues API calls and the enclave agent will see authorization failures with no obvious error.
💡 Suggestion
Derive the TTL from the same constants used for tool timeout to keep them in sync:
// enclaveGitHubProxyMaxTTL covers the full enclave execution window.
enclaveGitHubProxyMaxTTL = maxEnclaveTimingBucketSeconds + enclaveMCPTransportAllowance // 4860Also update the existing test assertion from the literal 600 to enclaveGitHubProxyMaxTTL so future constant changes are caught automatically:
assert.Equal(t, enclaveGitHubProxyMaxTTL, policy.MaxCapabilityTTL)If mcpg caps max_capability_ttl_seconds at a lower bound, document that bound here.
@copilot please address this.
| writeProxyUpstreamEnv(yaml) | ||
| fmt.Fprintf(yaml, " ENCLAVE_GITHUB_PROXY_IMAGE: %s\n", quoteYAMLEnvValue(containerImage)) | ||
| fmt.Fprintf(yaml, " %s: %s\n", enclaveGitHubProxyAliasEnv, enclaveGitHubProxyNetworkAlias) | ||
| fmt.Fprintf(yaml, " ENCLAVE_GITHUB_PROXY_POLICY_TEMPLATE: %s\n", quoteYAMLEnvValue(policyTemplate)) |
There was a problem hiding this comment.
[/codebase-design] The policy template JSON (including repository names and sensitivity labels) is emitted as a plaintext YAML step env: value. GitHub Actions renders the step environment block in the job log before the script runs, so ::add-mask::${POLICY_TEMPLATE} at line 33 of the start script does not protect values that appear in the compiled .lock.yml or in the pre-script log line.
💡 Why this matters
Repository slugs and sensitivity levels are DIFC-sensitive. A non-secret env var value is visible in both the generated lock file and the Actions run log. The current masking only prevents the value from leaking after the mask is registered, not at env-block rendering time.
The existing prior review comment on line 131 was marked resolved (commit c1c5da3d added ::add-mask:: calls inside the script), but the underlying issue — that the value is unmasked in the log when the step environment block is printed — remains.
Mitigations to consider:
- Pass the policy template through a GitHub Actions secret (e.g., write it to a job-scoped secret at compile time) — difficult for a compiler-owned value.
- Encode the template as a base64-opaque blob via
secrets.GITHUB_TOKEN-equivalent${{ toJSON(...) }}— not available for compiler-emitted env. - Emit the full policy generation (the
jqcall currently in the shell script) into the Go compiler so the template variable only ever holds a placeholder, and the final JSON is derived in-process from the separate token/run-ID at startup. The Go testTestGenerateEnclaveGitHubProxySetupalready assertsENCLAVE_GITHUB_PROXY_POLICY_TEMPLATE:is present, so this would need updating.
At minimum, document this in the security section of the enclave docs so operators are aware that repository names appear in their Actions logs.
@copilot please address this.
Summary
enclaves[].agent.github.cli: issues-read-v1syntax to the user and AWF schemasawf-enclave-github-proxySmoke-run fixes
Smoke run
32928731346exposed an ordering bug: the compiler's eager Start MCP Gateway functionality check treated the AWF-ownedawf-enclavebackend like an already-running required server. This PR classifies only the compiler-owned exact server nameawf-enclaveas deferred for that eager check. It remains required in mcpg configuration, the authenticated route remains fail-closed, arbitrary/user-selected names cannot be deferred, ordinary required failures remain fatal, and AWF still owns boundedinitializeplustools/listreadiness after attachment.Smoke run
32998355218then showed the maskedMCP_GATEWAY_API_KEYwas unavailable to the host AWF process. GitHub Actions may suppress secret-valued step outputs after::add-mask::, so enclave workflows now carry the already-masked key through the existing compiler-ownedGITHUB_ENVhost handoff. Enclave mount, execution, and cleanup steps inherit it instead of overriding it with the suppressed output. AWF still receives--exclude-env MCP_GATEWAY_API_KEY, so the primary agent cannot read the key, and the value is never placed in agent configuration or logs.Smoke run
33004295192proved the key handoff and enclave startup now work, then failed CA-backed hostname verification because mcpg v0.4.11 certificates covered only localhost/127.0.0.1 while AWF correctly connects throughawf-enclave-github-proxy. The compiler supplies that centralized fixed alias to the launcher and passes--tls-dns-name "$PROXY_ALIAS"alongside--tls. The proxy remains bridge-only with no published host port, and identity labels, masked handoffs, deferred MCP semantics, and primary-agent exclusions are unchanged.Validation
smoke-enclave-issues-read.mdcompilation: 1 succeeded, 0 warningsawf-enclaveroute, and primary-agent exclusionsmake agent-report-progressremains non-zero for the repository's existing function-length custom-linter baseline; this local worktree also reported a stale parallel golangci-lint runner. On macOS, the standalone MCP checker harness's healthy-server cases require GNUdate/head; shellcheck passes and the enclave-specific fail-closed/deferred cases pass.Dependencies
This remains a draft dependency-layer PR and should merge after the corresponding gh-aw-mcpg and AWF changes. AWF
v0.28.9and mcpgv0.4.12are published. AWF v0.28.9 includes the protected enclave agent startup/entrypoint diagnostics from gh-aw-firewall#7772; mcpg v0.4.12 includes the requiredissues-read-v1and repeatable--tls-dns-namecontracts.Pinned release images:
ghcr.io/github/gh-aw-firewall/agent:0.28.9@sha256:54b2fb3068efc15a4cc1bd4033f8fa056a9b1779baeba0cb80ae95ea55e7e343ghcr.io/github/gh-aw-firewall/api-proxy:0.28.9@sha256:a0ffb1dc926c6e5a500b336893e032a8f167d3db43c869be886874ef14280bb8ghcr.io/github/gh-aw-firewall/cli-proxy:0.28.9@sha256:38d7ac0585ee5aa6a06eb71e087d514b059db36005c7783c6485e0dfd36fea35ghcr.io/github/gh-aw-firewall/squid:0.28.9@sha256:3d5dba0b0a139bbb11b5d5b8b44f277d2b18f69cf43090e3c283d750cf864baaghcr.io/github/gh-aw-firewall/enclave-agent:0.28.9@sha256:8d548153f18d9b44406bebe3f71e2e080c15c89cbf8c0f72e8aa7bc0681efcf2ghcr.io/github/gh-aw-firewall/enclave-mcp-server:0.28.9@sha256:9edfa59fe0cf96f86c0be2f3280743a95032cf730526a204f729a06fbe7e4727ghcr.io/github/gh-aw-firewall/enclave-script:0.28.9@sha256:def4cc9669c0723cbdcdcdbd0ad4c72b6d977c02e4e0978797e9116fffd8e25dghcr.io/github/gh-aw-mcpg:v0.4.12@sha256:92d5377b6bd32cd5b9306b2a553f7ef3549bccff9207e46f931e7249bc718713The AWF v0.28.8 and v0.28.9 canonical config schema/spec blobs are identical, so no embedded schema or config-source spec update is required.